Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NIP-54 - Inline Resource Metadata #521

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

arthurfranca
Copy link
Contributor

It extracts the best from the previous discussion.

One additional upside is that when the regular user right clicks an image and copies then pastes the url on a new note, it preserves all added metadata. Also, it can be appended to any url like this NIP-21 one: nostr:nevent1qq...epm#a-metadata-name=a-metadata-value

@vitorpamplona
Copy link
Collaborator

vitorpamplona commented May 12, 2023

Sorry, why are we moving the description to alt on NIP-94? Do we need 2 descriptors?

@arthurfranca
Copy link
Contributor Author

arthurfranca commented May 12, 2023

@vitorpamplona It's because alt is basically for blind people, so it should be a correct description. While the other one (NIP-94 event content) could be a loose description, like a funny caption that not really describes the picture, just like html image titles.

edit: content is also a description (just like alt is), but I called it "caption" to try to differentiate it from alt tag

edit2:

Note: From an accessibility viewpoint, captions and alt text have distinct roles. Captions benefit even people who can see the image, whereas alt text provides the same functionality as an absent image. Therefore, captions and alt text shouldn't just say the same thing, because they both appear when the image is gone. Try turning images off in your browser and see how it looks.

@vitorpamplona
Copy link
Collaborator

vitorpamplona commented May 12, 2023

We used the .content as the descriptor for the blind. I don't know why there would be a need for another descriptor. It doesn't make much sense to me unless you are trying to do two things in a single event type.

@arthurfranca
Copy link
Contributor Author

[...] unless you are trying to do two things in a single event type.

It is exactly that. I imagined a photo gallery of NIP-94 kind 1063 events.

On one of the photos, the .content description says "This day at Copacabana was awesome!". In the abscence of assistive text (alt), you could use .content as such. But alt tag text of the same photo could be "A sunset at the beach with a group of four people looking at it". This has the benefit of not requiring kind 1 to have a photo caption and not showing up on kind 1 feed.

Now if you want this on regular social feed, the 1063 event would be referenced on a kind 1 note.

@vitorpamplona
Copy link
Collaborator

vitorpamplona commented May 12, 2023

So, the stories in Amethyst are exactly that. But without any additional descriptors. Users can only add the accessibility descriptor.

I understand that it makes things easier to not have another kind, but I am not sure if it's the right approach. We have to keep in mind that NIP-94 and NIP-95 are generalized file references and not posts. I believe another type is required to host them correctly if your goal is to make another non-kind1 feed.

Otherwise, you will start seeing 1063s that were included in replies in a Kind 1 on your image feed, disconnected from the Kind 1 context they were designed for. Or files that were added to a GitHub on Nostr project, or boring marketplace product pictures. If your feed is based on Kind 1063 alone (like ours is, for now) you will see a bunch of images and posts out of context.

And keep in mind, 1063 is for any file. Not only for images or videos.

@arthurfranca
Copy link
Contributor Author

Otherwise, you will start seeing 1063s that were included in replies in a Kind 1 on your image feed

Yes, but just put a t tag like photo_gallery or something and load 1063 only with that.

@arthurfranca
Copy link
Contributor Author

I consider caption part of metadata but I'm ok reverting the NIP-94 change. I will just wait if someone has anything to add.

@fiatjaf
Copy link
Member

fiatjaf commented May 13, 2023

It doesn't make much sense to have this NIP given that the Damus way already exists and is being used in practice. We should all use that. Yes, that's not the best way to write a spec ("just follow whatever Damus is doing") but that is probably the best way to make an existing protocol interoperable.

@arthurfranca arthurfranca force-pushed the inline-resource-metadata branch from 120b793 to c06cba6 Compare May 17, 2023 14:20
@arthurfranca
Copy link
Contributor Author

@vitorpamplona I reverted the change on .content so to not break your app. Added you to author cause you were the one pushing to using NIP-94 field names and percent-encoded fields.

@Giszmo is author of the # fragment idea.

I'm also OK with it in the url, I would just have to include it in both tags and URL until I am able to change the way I do content parsing.

@fiatjaf, the quote is from here by @jb55.

After re-reading the discussion, it seems both approaches are useful and "should" be used together. I highlighted the reason and differences here. The NIP also features a complete event example.

@fiatjaf
Copy link
Member

fiatjaf commented May 17, 2023

I don't see why we should have two approaches, that goes against all reason.

@arthurfranca
Copy link
Contributor Author

This is because imeta tag alone lacks some features.

Different from imeta tag, inlining with # fragment makes it possible to:

  • sharing an url on DM .content without leaking metadata;
  • can be used to extend any url on DM, no matter the protocol (even NIP-21 ones without adding new TLV types, like e.g.: nostr:nevent1...#any=a&metadata=b);
  • on web clients, user can right click/long tap on an rendered image (using default browser context menu)>copy url>paste url in another note keeping metadata

imeta tag is better for:

  • having dimension and blurhash at hand faster (especially if you are delaying .content parsing) so to prepare the image/video container.

@Giszmo
Copy link
Member

Giszmo commented May 18, 2023

Concept ACK. Skimmed it and looks good. Need some time to do a full review.

Copy link
Collaborator

@Semisol Semisol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will need to fully nitpick later

@jb55
Copy link
Contributor

jb55 commented Aug 22, 2023

LGTM

@arthurfranca arthurfranca force-pushed the inline-resource-metadata branch from eb02eda to 136ae8f Compare August 25, 2023 12:59
@arthurfranca
Copy link
Contributor Author

Just rebased, no changes

@arthurfranca
Copy link
Contributor Author

What if we do:
["r", "https://xyz.com/example.jpg#dim=3024x4032&alt=A%20scenic%20photo%20overlooking%20the%20coast%20of%20Costa%20Rica&blurhash=eVF%24%5EOI%3A%24%7BM%7Bo%23*0-nNFxakD-%3FxVM%7DWEWB%25iNKxvR-oetmo%23R-aen%24"]

Instead of:
["imeta", "url https://xyz.com/example.jpg", "dim 3024x4032", "alt A scenic photo overlooking the coast of Costa Rica", "blurhash eVF$^OI:${M{o#*0-nNFxakD-?xVM}WEWB%iNKxvR-oetmo#R-aen$"]

Positive:

  1. The same syntax for both in-line and on-tag media metadata.
  2. Makes NIP-34: Media Attachments #751 unnecessary.

Negative:

  1. Added percent-encoding to metadata values (minor performance hit)
  2. @jb55 Damus would need to change imeta to r tag

@alexgleason
Copy link
Member

IMO using URL fragments is not a good solution. See #751 which uses a JSON string.

@arthurfranca
Copy link
Contributor Author

arthurfranca commented Aug 31, 2023

Replaced imeta with resource tag (non-indexable) to keep same encoding (for both inside .content and instide tag URLs). Also added some tag examples:

A kind 30023 event JSON file (with nostr/30023 MIME type):
["resource", "https://xyz.com/example.json#m=nostr%2F30023"]

A PDF file without extension with added MIME type information:
["r", "https://xyz.com/article02#m=application%2Fpdf"]

@arthurfranca
Copy link
Contributor Author

@alexgleason your solution 1) force tag usage that may not be suitable to current DMs/encrypted events (metadata leakage). 2) make copy/pasting urls lose metadata 3) force kind 1/30023 clients showing events to not only parse .content (looking for mentions/urls/hashtags) but also parse tags.

⚔️ The clash of the url metadata PRs

@alexgleason
Copy link
Member

Most clients don't display inline media anyway, they pull it down to the bottom. Enumerating tags instead is an easier solution to what they're already doing.

Leaking metadata in DMs is already an issue, and it's why giftwrap is being proposed. The tags would be hidden with giftwrap.

@arthurfranca
Copy link
Contributor Author

Most clients, not all of them

@arthurfranca
Copy link
Contributor Author

arthurfranca commented Aug 31, 2023

@alexgleason This note on Amethyst gets the image inlined at the intended position: https://nostr.com/nevent1qqsdnu95d6lxct2k6nx5nprauwhauchad544sctdjmrtfv33y334lfgpz3mhxue69uhhyetvv9ujumn0wd68ytnzvupzq3huhccxt6h34eupz3jeynjgjgek8lel2f4adaea0svyk94a3njdqvzqqqqqqyvseq6w

edit: In fact, of all the clients listed at nostr.com, 5 were inlined at correct line (not at bottom). 1 turned the URL into a link. 1 didn't find it and the other 2 didn't load. Of the mobile clients I know, Amethyst inlines it while Damus and plebstr put it at the bottom.

@alexgleason
Copy link
Member

Amethyst does everything under the sun currently possible on Nostr. It is most prepared to deal with things both ways, while the majority of clients just want to stick media at the bottom.

@jb55
Copy link
Contributor

jb55 commented Dec 12, 2023

🤮 they should fix that. I personally switched from nostr.build from nostrcheck for exactly this reason. Tidy urls are nice. But maybe I'm a nerd.

would have to strip the # if metadata is detected I guess

@vitorpamplona
Copy link
Collaborator

vitorpamplona commented Dec 12, 2023

weird to me you would give up metadata from one of the largest clients over these minor differences, but alright. I guess we have two competing standards in the wild now.

Nothing personal. I go with what's best at the time. If somebody creates a better one, I am happy to migrate.

@vitorpamplona
Copy link
Collaborator

vitorpamplona commented Dec 12, 2023

BTW, For completeness, I also implemented shoving the entire NIP-94 event in a tag as a serialized json for each URL and then using a nostr:nevent... to point to the image in the .content instead of the URL.

It's ok. But it ended up still URL-based (it just transferred the limitations of dealing with a URL to the NIP-94 event). And since this PR allows the use of any nip-94 tag in the fragment, this version is just shorter.

@alexgleason
Copy link
Member

alexgleason commented Dec 21, 2023

would have to strip the # if metadata is detected I guess

function renderLink(href: string): string {
  const url = new URL(href);
  const search = new URLSearchParams(url.search);

  if (isMediaParams(search)) {
    url.hash = '';
  }

  return `<a href="${url.href}">${url.href}</a>`;
}

function isMediaParams(search: URLSearchParams): boolean {
  // how am I supposed to detect this?
  return search.has('m'); // ?
}

@alexgleason
Copy link
Member

You can't remove the hash unconditionally, otherwise people can't post links like: https://en.wikipedia.org/wiki/Nostr#See_also

@vitorpamplona
Copy link
Collaborator

vitorpamplona commented Dec 21, 2023

You should remove it just for display purposes, not from the actual link. I get rid of everything and just keep the host and some of the path.

@alexgleason
Copy link
Member

I don't think it's a good solution to hide the hash from URLs.

@vitorpamplona
Copy link
Collaborator

I don't think it's a good solution to hide the hash from URLs.

You mean the file at the end of the path?

For instance, for

https://i.nostr.build/n5AQ.jpg#m=image%2Fjpeg&dim=1080x1626&blurhash=%5EE986*Z%24DQr.nMkY8w%25f-mWtS%24ngvfoecEn*r%3Fav.mVtMxs8WTo%7ED*t6%24eS%24g4aL%3FEb0K6v%7DnNRiIBxY%24%25bdW%3Bxtx%5Br%5Daeoet6Rk%252kDWYi%5EV%40j%3F&x=4d9ad2dd6635fc937427ed0fbb723c0b925d1e0b304b925a65a1185581596dfe

I remove the https:// scheme, all query params, fragments and just show i.nostr.build/n5AQ.jpg.

For long hashes, like:

https://image.nostr.build/10f7dc11a35d6fb0f8372666e6392287cd272d543a3832fb3e5796a547d83172.jpg#m=image%2Fjpeg&dim=1920x1440&blurhash=%23BGHuG0K%23R%7Ep9F%25zr%3Dxtxuv%7D%7EV9ar%3Dx%5DD%25tRi%7BkC00m-cE%24fNxn%2CkCR*o001E1-%3BEMsmtRVskWsmxaXmr%3FIp%250j%40RjxZSN%7EWtQR5WXjYRPozRjRktQs%2BRjWBofV%40R*WCV%5B&x=fd8b2438b21bfc35a560c1ac6496f8c3fc8cca085ebd734d98ecd7ac22279df3

I add a ... between the host and the end of the path:

image.nostr.build/...a547d83172.jpg

Assuming of course that I can't preview it for some reason.

@staab
Copy link
Member

staab commented Dec 21, 2023

Hashes also seems to break imgproxy. Still NACK on this, imeta is better.

@vitorpamplona
Copy link
Collaborator

Hashes also seems to break imgproxy. Still NACK on this, imeta is better.

Any examples of the issue? If it is breaking in these simple cases, we better fix it before the real content servers come in.

@staab
Copy link
Member

staab commented Dec 21, 2023

Oh, nevermind, the bug was my own

@v0l
Copy link
Member

v0l commented Dec 21, 2023

Snort is using the imeta tag i don't link these long URL's

@jb55
Copy link
Contributor

jb55 commented Dec 21, 2023

some of the links seem to have evaded my url parser, leading to broken images in a few cases 😢

@Yonle
Copy link

Yonle commented Dec 26, 2023

Most image parser will expecting .jpg, .png, .gif, or similiar at the end of the URL. But this completely break the parsee that most nostr client use.

@fiatjaf
Copy link
Member

fiatjaf commented Dec 26, 2023

I think this was a good try but the big URLs create a big burden on everybody and we should stop.

I guess it's correct to say that the URL parsers and regexes should have been ready for this, but in practice in the real world no one ever does such big URLs with # and * and { and whatnot, and when they do it's not a big deal if they fail to get recognized, but now it's thousands of notes with them and their authors don't even know they're publishing such giant monsters.

@Yonle
Copy link

Yonle commented Dec 26, 2023

There are even posts that put .mp4 (or similiar at the end of url) after # for attachments.

@Yonle
Copy link

Yonle commented Dec 26, 2023

boy, do we need to sacrifice (end) user experience for a feature?

@vitorpamplona
Copy link
Collaborator

Most image parser will expecting .jpg, .png, .gif, or similiar at the end of the URL.

This is the dumbest way to check what the content type of a URL is. There are no file extensions in URIs. Some things look like them that show up in URIs, but that's only because some servers map URIs to files internally. They are meaningless for anything outside those servers.

The only way to reliably establish if a resource identified by a particular URI is an image or not is to make a HEAD or GET request and look at the Content-Type response header.

Take the famous site 500px, for instance. None of their photo URLs end with an extension. https://drscdn.500px.org/photo/79694843/m%3D2048_k%3D1_a%3D1/v2?v=0&sig=5cd241cf922efd12d8aa1dac2722e261b006648cd0b7e3902bb6f4360e17d450

If you are relying on extensions, you are offering a terrible user experience already.

@Yonle
Copy link

Yonle commented Dec 26, 2023

Most image parser will expecting .jpg, .png, .gif, or similiar at the end of the URL.

This is the dumbest way to check what the content type of a URL is. There are no file extensions in URIs. Some things look like them that show up in URIs, but that's only because some servers map URIs to files internally. They are meaningless for anything outside those servers.

Take the famous site 500px, for instance. None of their photo URLs end with an extension. https://drscdn.500px.org/photo/79694843/m%3D2048_k%3D1_a%3D1/v2?v=0&sig=5cd241cf922efd12d8aa1dac2722e261b006648cd0b7e3902bb6f4360e17d450

If you are relying on extensions, you are offering a terrible user experience already.

The thing is that most nostr clients did it this way for quickly recognizing the URL as an attachment.

@vitorpamplona
Copy link
Collaborator

vitorpamplona commented Dec 26, 2023

The thing is that most nostr clients did it this way for quickly recognizing the URL as an attachment.

Sure. They can keep doing it. Just remove everything after ? and after # before checking. It will work for the images from this PR, but it won't work for many image servers out there.

@fiatjaf
Copy link
Member

fiatjaf commented Dec 27, 2023

If you are relying on extensions, you are offering a terrible user experience already.

We should standardize and enforce that extensions are required for media content.

@vitorpamplona
Copy link
Collaborator

vitorpamplona commented Dec 27, 2023

We should standardize and enforce that extensions are required for media content.

Why? We have NIP-94, imeta and this PR to offer proper content types. Bringing back a flimsy outdated file extension model from the 90s on top of them just duplicates everything yet again.

Keep in mind that clients can't add extensions on links that don't already offer them. There is not much we can do.

If we want to truly fix this, we should not use URLs at all. Everything else is just a half-ass hack.

@fiatjaf
Copy link
Member

fiatjaf commented Dec 27, 2023

If we want to truly fix this, we should not use URLs at all. Everything else is just a half-ass hack.

I agree, but Nostr itself is a half-ass hack. The URL system is very broken and I wanted to fix it too, but is it worth fixing? We have tradeoffs here at hand. If we can get a system that works it will be better than one that is perfect but doesn't work because it's too complex, has too much indirection, slowness.

Why? We have NIP-94, imeta and this PR to offer proper content types. Bringing back a flimsy outdated file extension model from the 90s on top of them just duplicates everything yet again.

Keep in mind that clients can't add extensions on links that don't already offer them. There is not much we can do.

But yeah, maybe let's not try to force-standardize this for now, since it wouldn't work. But I still think people should try to put extensions in URLs whenever they can, as a kind gesture to the dumber clients that rely on that flawed hackish property of URLs.

@vitorpamplona
Copy link
Collaborator

vitorpamplona commented Dec 27, 2023

is it worth fixing?

Yes. The decentralization benefits alone are a huge win. And clients get a better set of preview tags to optimize their UX before the content is loaded. It's a win-win situation.

This feels very similar to the NIP-04 discussion. Everybody knew it was wrong but complacency and laziness kept us stuck in that model for far too long.

Sometimes the easiest design and the simplest design are vastly different from one another. This is one of those cases. We can keep piling into an easy design the URL overlords want us to use or design our own content naming and resolution scheme to make it 10x better (more aligned with Nostr needs).

In the end, our design will be simpler to implement than trying to make URLs work in 100% of the cases.

@arthurfranca
Copy link
Contributor Author

Most image parser will expecting .jpg, .png, .gif, or similiar at the end of the URL. But this completely break the parsee that most nostr client use.

You telling me kind:1 client devs that are already dealing with decentralized relay quirks and other harder NIPs are incapable of updating an url regexp to a valid one? =o

If #... is breaking them, ?... was already breaking these parsers too

@jb55
Copy link
Contributor

jb55 commented Dec 28, 2023 via email

@arthurfranca
Copy link
Contributor Author

It works alright no need to change. Clients will only fail to load an image/video if the parser use some poor regex not accounting for valid urls (that may include ? query params or # hash) and this has nothing to do with this NIP because the file extension is not stripped from the url.

some of the links seem to have evaded my url parser

Probably just media with no extension. Nothing related to this NIP.

On NIP-96 i have added the advice to append the media extension to urls so that nip54 or imeta isnt required to detect mime type.

Nip54 is totally optional and beautifying long urls or not when no media is detected is a dev choice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.